-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[v2.11] Backport: Enable type-safe interoperability between different independent Python/C++ bindings systems. #5370
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…n/C++ bindings systems. (pybind#5296) * `self.__cpp_transporter__()` proof of concept: Enable passing C++ pointers across extensions even if the `PYBIND11_INTERNALS_VERSION`s do not match. * Include cleanup (mainly to resolve PyPy build failures). * Fix clang-tidy errors. * Resolve `error: extra * factor out platform_abi_id.h from internals.h (no functional changes) * factor out internals_version.h from internals.h (no functional changes) * Update CMakeLists.txt, tests/extra_python_package/test_files.py * Revert "factor out internals_version.h from internals.h (no functional changes)" This reverts commit 3ccea8c. * Remove internals_version.h from CMakeLists.txt, tests/extra_python_package/test_files.py * `.__cpp_transporter__()` implementation: compare `pybind11_platform_abi_id`, `cpp_typeid_name` * Add PremiumTraveler * Rename test_cpp_transporter_traveler_type.h -> test_cpp_transporter_traveler_types.h * Expand tests: `PremiumTraveler`, `get_points()` * Shuffle order of tests (no real changes). * Move `__cpp_transporter__` lambda to `py::cpp_transporter()` regular function. * Use `type_caster_generic::load(self)` instead of `cast<T *>(self)` * Pass `const std::type_info *` via `py::capsule` (instead of `cpp_typeid_name`). * Make platform_abi_id.h completely stand-alone. * rename exo_planet.cpp -> exo_planet_pybind11.cpp * Add exo_planet_c_api.cpp (incomplete). * Fix silly oversight (wrong filename in `#include`). * Resolve clang-tidy errors: ``` /__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:10:18: error: 'wrapGetLuggage' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors] 10 | static PyObject *wrapGetLuggage(PyObject *, PyObject *) { return PyUnicode_FromString("TODO"); } | ~~~~~~ ^ /__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:14:20: error: 'ThisMethodDef' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors] 14 | static PyMethodDef ThisMethodDef[] | ~~~~~~ ^ /__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:17:27: error: 'ThisModuleDef' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors] 17 | static struct PyModuleDef ThisModuleDef = { | ~~~~~~ ^ ``` * Implement exo_planet_c_api GetLuggage(), GetPoints() * Move new code from test_cpp_transporter_traveler_bindings.h to pybind11/detail/type_caster_base.h, under the name `class_dunder_cpp_transporter()` * Fix oversight. * Unconditionally add `__cpp_transporter__` method to all `py::class_` objects, but do not include that magic method in docstring signatures. * Back out pybind11/detail/platform_abi_id.h for now. Maximizing reusability can be handled separately, later. * Small cleanup. * Restore and add to `test_call_cpp_transporter_*()` * Ensure pybind#3788 does not bite again. * `class_dunder_cpp_transporter()`: replace `obj.cast<std::string>()` with `std::string(obj)` * Add (simple) copyright notices in all newly added files. * Globally replace cpp_transporter with cpp_conduit * style: pre-commit fixes * IWYU fixes * Rename `class_dunder_cpp_conduit()` -> `cpp_conduit_method()` * Change `pybind11_platform_abi_id`, `pointer_kind` argument types from `str` to `bytes`. This avoids the unicode decode/encode roundtrips: * More robust (no decode/encode errors). * Minor runtime optimization. * Systematically rename `cap_cpp_type_info` -> `cpp_type_info_capsule` (no functional changes). * Systematically replace `cpp_type_info_capsule` `name`: `"const std::type_info *"` -> `typeid(std::type_info).name()` (this IS a functional change). This provides an extra layer of protection against C++ ABI mismatches: * The first and most important layer is that the `PYBIND11_PLATFORM_ABI_ID`s must match between extensions. * The second layer is that the `typeid(std::type_info).name()`s must match between extensions. * Fix sort order accident in tests/CMakeLists.txt * Apply suggestions from code review Co-authored-by: Aaron Gokaslan <aaronGokaslan@gmail.com> * style: pre-commit fixes * refactor: rename to _pybind_conduit_v1_ Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com> * Add test_home_planet_wrap_very_lonely_traveler(), test_exo_planet_pybind11_wrap_very_lonely_traveler() * Resolve clang-tidy errors: ``` /__w/pybind11/pybind11/tests/test_cpp_conduit_traveler_bindings.h:39:32: error: parameter 'm' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param,-warnings-as-errors] 10 | py::class_<LonelyTraveler>(m, "LonelyTraveler"); | ^ | std::move( ) /__w/pybind11/pybind11/tests/test_cpp_conduit_traveler_bindings.h:43:52: error: parameter 'm' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param,-warnings-as-errors] 43 | py::class_<VeryLonelyTraveler, LonelyTraveler>(m, "VeryLonelyTraveler"); | ^ | std::move( ) ``` --------- Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com> Co-authored-by: Ralf W. Grosse-Kunstleve <rwgk@google.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com> Co-authored-by: Aaron Gokaslan <aaronGokaslan@gmail.com>
Squash-merging, after coordinating with @henryiii on slack. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Backport of PR #5296 to the pybind11 v2.11 series, to maximize the benefit of the new cross-version interoperability feature.
Suggested changelog entry: